-
Notifications
You must be signed in to change notification settings - Fork 321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NET-6204- Repeating error log in consul-connect-injector #3128
Conversation
no error after 5 minutes of consul server running |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
// if we timeout we don't care about the error message because it's expected to happen on long polls | ||
// any other error we want to alert on | ||
if !strings.Contains(strings.ToLower(err.Error()), "timeout") && | ||
!strings.Contains(strings.ToLower(err.Error()), "no such host") && | ||
!strings.Contains(strings.ToLower(err.Error()), "connection refused") { | ||
r.logger.Error(err, fmt.Sprintf("unable to fetch config entry for gateway: %s/%s", ref.Namespace, ref.Name)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wish we had a better way to do error handling in general, but this matches what we're doing elsewhere
consul-k8s/control-plane/api-gateway/cache/consul.go
Lines 196 to 206 in 2d03f3e
entries, meta, err := client.ConfigEntries().List(kind, opts.WithContext(ctx)) | |
if err != nil { | |
// if we timeout we don't care about the error message because it's expected to happen on long polls | |
// any other error we want to alert on | |
if !strings.Contains(strings.ToLower(err.Error()), "timeout") && | |
!strings.Contains(strings.ToLower(err.Error()), "no such host") && | |
!strings.Contains(strings.ToLower(err.Error()), "connection refused") { | |
c.logger.Error(err, fmt.Sprintf("error fetching config entries for kind: %s", kind)) | |
} | |
continue | |
} |
Changes proposed in this PR:
-dont error on timeouts in gateway
How I expect reviewers to test this PR:
Setup a consul server and let it sit for >5 minutes, we should not see this error pop up
Checklist: